Review checklist

This checklist is meant to assist creators of PRs (to let them know what reviewers will typically look for) and reviewers (to guide them in a structured review process). Items do not need to be checked explicitly for a PR to be eligible for merging.

Purpose and scope

• The PR has a single goal that is clear from the PR title and/or description.
• All code changes represent a single set of modifications that logically belong together.
• No more than 500 lines of code are changed or there is no obvious way to split the PR into multiple PRs.

Code quality

• The code can be understood easily.
• Newly introduced names for variables etc. are self-descriptive and consistent with existing naming conventions.
• There are no redundancies that can be removed by simple modularization/refactoring.
• There are no leftover debug statements or commented code sections.
• The code adheres to our conventions and style guide, and to the Julia guidelines.

Documentation

• New functions and types are documented with a docstring or top-level comment.
• Relevant publications are referenced in docstrings (see example for formatting).
• Inline comments are used to document longer or unusual code sections.
• Comments describe intent ("why?") and not just functionality ("what?").
• If the PR introduces a significant change or new feature, it is documented in NEWS.md with its PR number.

Testing

• The PR passes all tests.
• New or modified lines of code are covered by tests.
• New or modified tests run in less then 10 seconds.

Performance

• There are no type instabilities or memory allocations in performance-critical parts.
• If the PR intent is to improve performance, before/after time measurements are posted in the PR.

Verification

• The correctness of the code was verified using appropriate tests.
• If new equations/methods are added, a convergence test has been run and the results
  are posted in the PR.

Created with ❤️ by the Trixi.jl community.

Codecov Report

Attention: Patch coverage is 99.29701% with 4 lines in your changes missing coverage. Please review.

Project coverage is 96.43%. Comparing base (f45455b) to head (d26713e).
Report is 2 commits behind head on main.

@@            Coverage Diff             @@
##             main    #2196      +/-   ##
==========================================
+ Coverage   96.39%   96.43%   +0.04%     
==========================================
  Files         483      485       +2     
  Lines       38349    38918     +569     
==========================================
+ Hits        36964    37529     +565     
- Misses       1385     1389       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

The old performance issues have been resolved. For comparison, the performance indices on a single core of an AMD Ryzen Threadripper 3990X for the test examples/tree_2d_dgsem/elixir_mhdmultiion_ec.jl, which has 2 ion species and 14 equations, are about

time/DOF/rhs!:  6.64871945e-07 s, and `PID:            6.89436731e-07 s

These performance indices are comparable with the performance indices of the single-fluid MHD test ‘examples/tree_2d_dgsem/elixir_mhd_ec.jl‘:

time/DOF/rhs!:  1.50055863e-07 s, and PID:            1.66041059e-07 s

Here is the summary at the end of the 2-ion simulation:

────────────────────────────────────────────────────────────────────────────────────
             Trixi.jl                      Time                    Allocations      
                                  ───────────────────────   ────────────────────────
        Tot / % measured:              1.44s /  98.8%           34.5MiB /  98.0%    

Section                   ncalls     time    %tot     avg     alloc    %tot      avg
────────────────────────────────────────────────────────────────────────────────────
rhs!                         491    1.32s   92.8%  2.69ms   9.33KiB    0.0%    19.5B
  volume integral            491    1.06s   74.6%  2.16ms     0.00B    0.0%    0.00B
  interface flux             491    177ms   12.4%   360μs     0.00B    0.0%    0.00B
  source terms               491   43.5ms    3.1%  88.6μs     0.00B    0.0%    0.00B
  surface integral           491   17.5ms    1.2%  35.6μs     0.00B    0.0%    0.00B
  prolong2interfaces         491   14.3ms    1.0%  29.1μs     0.00B    0.0%    0.00B
  Jacobian                   491   3.59ms    0.3%  7.31μs     0.00B    0.0%    0.00B
  reset ∂u/∂t                491   2.89ms    0.2%  5.89μs     0.00B    0.0%    0.00B
  ~rhs!~                     491    538μs    0.0%  1.10μs   9.33KiB    0.0%    19.5B
  prolong2boundaries         491   24.6μs    0.0%  50.0ns     0.00B    0.0%    0.00B
  prolong2mortars            491   20.7μs    0.0%  42.2ns     0.00B    0.0%    0.00B
  mortar flux                491   18.9μs    0.0%  38.5ns     0.00B    0.0%    0.00B
  boundary flux              491   11.2μs    0.0%  22.7ns     0.00B    0.0%    0.00B
analyze solution              11   55.2ms    3.9%  5.02ms   27.0MiB   79.6%  2.45MiB
calculate dt                  99   33.3ms    2.3%   336μs     0.00B    0.0%    0.00B
I/O                            6   13.6ms    1.0%  2.27ms   6.88MiB   20.3%  1.15MiB
  save solution                5   12.7ms    0.9%  2.54ms   6.84MiB   20.2%  1.37MiB
  ~I/O~                        6    899μs    0.1%   150μs   34.1KiB    0.1%  5.69KiB
  get element variables        5   2.00μs    0.0%   400ns     0.00B    0.0%    0.00B
  save mesh                    5   1.28μs    0.0%   256ns     0.00B    0.0%    0.00B
  get node variables           5    220ns    0.0%  44.0ns     0.00B    0.0%    0.00B
────────────────────────────────────────────────────────────────────────────────────

Please also add some unit tests for type stability in https://github.com/trixi-framework/Trixi.jl/blob/main/test/test_type.jl

Many thanks for the speedy reviews @sloede, @ranocha, @DanielDoehring !! 🙂

I have added unit tests for type stability in 79b0e29.

@sloede there were some (admittedly minor) unresolved conversations (three comments from me and one comment from Hendrik).

@sloede there were some (admittedly minor) unresolved conversations (three comments from me and one comment from Hendrik).

I have implemented the minor suggestions that were blocking the merge of this (already approved) PR in the commit 8af2059 of my new PR #2204.